Skip to content

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Dec 17, 2024

No description provided.

@sbc100 sbc100 requested review from aheejin and kripken December 17, 2024 02:39
parser = argparse.ArgumentParser()
parser.add_argument('-s', '--skip-tests', action='store_true', help="don't actually run the tests, just analyze the existing results")
parser.add_argument('-b', '--new-branch', action='store_true', help='create a new branch containing the updates')
parser.add_argument('-c', '--clear-cache', action='store_true', help='clear the cache before rebaselining (useful when working with llvm changes)')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be the default perhaps?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think most of the time this script will be used to update expectations in response for local emscripten changes (i.e. minor changes to JS libraries) not upstream llvm change. At least for me I often run rebases without clearing the cache when I know llvm has not changed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also makes it much slower if you clear the cache first, and when this script runs in CI it would never be needed (since CI libs are always built using tot anyway).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. lgtm either way.

@sbc100 sbc100 merged commit 916b34e into emscripten-core:main Dec 17, 2024
4 of 13 checks passed
@sbc100 sbc100 deleted the rebaseline_script branch December 17, 2024 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants